Skip to content

Add platform-specific default paths for Poetry cache and virtualenvs#1218

Merged
karthiknadig merged 2 commits intomainfrom
delicious-coyote
Feb 19, 2026
Merged

Add platform-specific default paths for Poetry cache and virtualenvs#1218
karthiknadig merged 2 commits intomainfrom
delicious-coyote

Conversation

@karthiknadig
Copy link
Member

Fixes #1182
Fixes #1184

@karthiknadig karthiknadig added the bug Issue identified by VS Code Team member as probable bug label Feb 13, 2026
@karthiknadig karthiknadig requested a review from Copilot February 13, 2026 18:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Poetry environment discovery to use platform-correct default cache/virtualenvs locations and adds logic to resolve Poetry’s {cache-dir} placeholder (addressing issues #1182 and #1184).

Changes:

  • Add getDefaultPoetryCacheDir() / getDefaultPoetryVirtualenvsPath() and use them as fallbacks for Poetry virtualenv discovery.
  • Add {cache-dir} placeholder resolution via poetry config cache-dir.
  • Add isMac() helper and unit tests for the new default-path helpers.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/managers/poetry/poetryUtils.ts Implements platform-specific defaults and {cache-dir} resolution for Poetry virtualenv paths.
src/common/utils/platformUtils.ts Adds isMac() platform helper.
src/test/managers/poetry/poetryUtils.unit.test.ts Adds unit tests for the new default Poetry cache/virtualenv path helpers.
Comments suppressed due to low confidence (2)

src/managers/poetry/poetryUtils.ts:210

  • resolveVirtualenvsPath() can return the original string containing {cache-dir} when it can’t resolve the placeholder. getPoetryVirtualenvsPath() then persists that unresolved value to workspace state, which can permanently break discovery until the state is cleared. Consider returning undefined (or falling back to getDefaultPoetryVirtualenvsPath()) when the placeholder can’t be resolved, and only caching paths that are resolved (no {…}) and absolute.

This issue also appears on line 303 of the same file.

                // Poetry might return the path with placeholders like {cache-dir}
                // Resolve the placeholder if present
                if (venvPath.includes('{cache-dir}')) {
                    poetryVirtualenvsPath = await resolveVirtualenvsPath(poetry, venvPath);
                } else if (path.isAbsolute(venvPath)) {
                    poetryVirtualenvsPath = venvPath;
                } else {
                    // Not an absolute path and no placeholder, use platform-specific default
                    poetryVirtualenvsPath = getDefaultPoetryVirtualenvsPath();
                }

                if (poetryVirtualenvsPath) {
                    await state.set(POETRY_VIRTUALENVS_PATH_KEY, poetryVirtualenvsPath);
                    return poetryVirtualenvsPath;
                }

src/managers/poetry/poetryUtils.ts:311

  • The “last resort” branch returns virtualenvsPath unchanged even though it still contains {cache-dir} and is likely unusable. Since callers treat a non-empty string as valid and may cache it, it would be safer to return undefined/throw, or to fall back to getDefaultPoetryVirtualenvsPath() so the rest of the code doesn’t propagate an invalid path.
    // Fall back to platform-specific default cache dir
    const defaultCacheDir = getDefaultPoetryCacheDir();
    if (defaultCacheDir) {
        return virtualenvsPath.replace('{cache-dir}', defaultCacheDir);
    }

    // Last resort: return the original path (will likely not be valid)
    return virtualenvsPath;
}

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/test/features/projectManager.initialize.unit.test.ts:352

  • Same formatting issue here: add spaces inside the braces for readability (or revert to brace-less one-liners if preferred).
                if (key === 'pythonProjects') {return [] as unknown as T;}
                if (key === 'defaultEnvManager') {return 'ms-python.python:venv' as T;}
                return defaultValue;

@karthiknadig karthiknadig marked this pull request as ready for review February 19, 2026 00:29
@karthiknadig karthiknadig enabled auto-merge (squash) February 19, 2026 00:29
@vs-code-engineering vs-code-engineering bot added this to the February 2026 milestone Feb 19, 2026
@karthiknadig karthiknadig merged commit 0b9d327 into main Feb 19, 2026
80 checks passed
@karthiknadig karthiknadig deleted the delicious-coyote branch February 19, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Poetry: {cache-dir} placeholder not properly resolved in virtualenvs.path Poetry: Incorrect default virtualenvs path on Windows and macOS

2 participants

Comments